Issue #1285 grid data from imod5#1286
Conversation
| data["area"] = get_cell_area_from_imod5_data(imod5_data) | ||
| data["landuse"] = get_landuse_from_imod5_data(imod5_data) | ||
| data["rootzone_depth"] = get_rootzone_depth_from_imod5_data(imod5_data) | ||
| data["surface_elevation"] = imod5_data["cap"]["surface_elevation"] |
There was a problem hiding this comment.
where does "cap" stand for?
There was a problem hiding this comment.
The "CAP" package in iMOD5, which I think refers to the "capillary zone".
| @classmethod | ||
| def from_imod5_data( | ||
| cls, | ||
| imod5_data: dict[str, dict[str, GridDataArray]], |
There was a problem hiding this comment.
It seems like they only thing we are using from imod5_data is the cap entry.
You could have this method accept only that entry
from_imod_data(data["cap"] ,...)
If thats not convienient for the user you could declare a local variable to hold it and pass only that to the other internal methods:
imod5_cap = imod5_data["cap"]
data = {}
data["area"] = get_cell_area_from_imod5_data(imod5_cap )
data["landuse"] = get_landuse_from_imod5_data(imod5_cap )
data["rootzone_depth"] = get_rootzone_depth_from_imod5_data(imod5_cap )
data["surface_elevation"] = imod5_cap["surface_elevation"]
There was a problem hiding this comment.
Good suggestion, done!
|
|
||
|
|
||
| def get_cell_area_from_imod5_data( | ||
| imod5_data: dict[str, dict[str, GridDataArray]], |
There was a problem hiding this comment.
The structure dict[str, GridDataArray] is something we use a lot. We could create a type alias to make working with it easier and also make it clearer from the type name what it is.
I was thinking about naming it something like
GridDataSet = dict[str, GridDataArray]
or
GridDataCollection = dict[str, GridDataArray]
There was a problem hiding this comment.
Good suggestion, GridDataset is already taken for:
GridDataset: TypeAlias = Union[xr.Dataset, xu.UgridDataset]I named a type alias: GridDataDict.
| imod5_data: dict[str, dict[str, GridDataArray]], | ||
| ) -> GridDataArray: | ||
| rural_landuse = imod5_data["cap"]["landuse"] | ||
| # Urban landuse = 18 |
There was a problem hiding this comment.
iMOD5 supports having two landuses per grid cell (so two "subunits" in iMOD Python lingo), the first one is for rural landuse, the second for urban landuse. The first one is specified by a rural landuse grid, and can thus have different values (e.g. agriculture, forestry, cattle), the second one is always landuse "urban", which is specified with the number 18.
I will explain some more in docstring.
|
|
Tests fail due to an issue with a single TeamCity agent, merging to feature branch. |
2 participants
Fixes #1285
Description
GridData.from_imod5_data, which has the following requirements:test_grid_data.pyto separate cases, reduces code duplicationGridData.from_imod5_dataIt has a arguments for regridding, but these don't do anything yet. I wasn't sure if I should keep this to already have a future-proof signature, or not add these, as the regridder arguments only confuse people. I don't think regridding the MetaSWAP iMOD5 grids (like we do with MODFLOW6 data) is a part of iMOD Python now, as we are first focusing on the LHM, for which this is not a hard requirement.
Checklist
Issue #nr, e.g.Issue #737